Skip to content

fix: apply value-sync auto-resolutions to the block the engine receives#2968

Merged
christianhg merged 1 commit into
mainfrom
fix-sync-autoresolve-repair
Jul 22, 2026
Merged

fix: apply value-sync auto-resolutions to the block the engine receives#2968
christianhg merged 1 commit into
mainfrom
fix-sync-autoresolve-repair

Conversation

@christianhg

@christianhg christianhg commented Jul 10, 2026

Copy link
Copy Markdown
Member

Probing the value sync's repair paths (follow-up territory from #2965) surfaced this: send an editor a mid-session update value whose changed block has a child missing its _key, and the editor ends up holding that keyless child, a shape its own addressing model can't represent (React immediately warns about duplicate keys rendering it), while the document receives a repair patch the editor never applied to itself.

The mechanism: validateValue runs at sync ingress and can auto-resolve certain defects (child missing _key, empty children, unused markDefs). But the resolution existed only as patches. The changed-block path emitted them outbound (gated to mid-session, non-readOnly, with a console.warn) while passing the raw block into updateBlock/replaceBlock; the insert path computed them and dropped them entirely. So the repair forked: the document got the resolution's minted key, the engine got the un-repaired block, and engine normalization later minted a different key for the same node. The old test pins are the fossil record of this double mint, pteWarningsSelfSolving expected k3, the key normalization minted after validateValue's k2 was discarded.

The fix applies the resolution to the block before it enters the engine, at both call sites, via applyAll from @portabletext/patches (the same constructors validateValue builds its resolutions with). The emitted patches are byte-identical to before and now agree with engine state: one key, minted once, on both sides. Emission semantics are deliberately untouched, the insert path still emits nothing, the changed path still emits only mid-session for non-readOnly editors; changing when repairs are emitted is the pending-events design work, tracked separately.

Three regression scenarios in event.update-value.test.tsx pin the contract: a mid-session keyless-child update repairs once (emitted patch key === engine key, exact values) and the sync survives to apply a subsequent update; a mid-session update with an unused markDef repairs on both sides, where the pre-fix red exposed a second defect of the fork: the repair was doubly emitted, the ingress keyed unset plus the engine normalizer's whole-array set for the same def, two conflicting patch shapes for one repair; and a startup value with a keyless child is repaired in the engine while emitting nothing (no mutation leaves a pristine editor on open). Negative assertions use a causal sentinel (one local edit, then a full-literal assert of the entire emission history) instead of sleeps, so a would-be emission is caught by ordering, not timing. The survival assertion needs a beat over a second: the sync machine parks in busy while its own emitted mutation flushes and re-polls on a 1s timer, which is also worth naming because an earlier probe mistook that delay for a permanent wedge; there is no wedge, the machine recovers by design.

Semantic deltas, all deliberate and pinned: repaired blocks now carry the first mint's key (pteWarningsSelfSolving, container-normalization pins shift from the second mint to the first), and an empty-children block arrives with its placeholder span inside the block's own insert operation instead of as a separate normalization fix op (event.operation). The normalization-adjacency contract that scenario used to exercise is re-pinned on duplicate child keys, which have no ingress check and are still repaired by normalization, so the contract keeps a living fixture. Full unit (852) and browser (1718) suites pass.

@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c2e6232

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 14 packages
Name Type
@portabletext/editor Patch
@portabletext/plugin-character-pair-decorator Patch
@portabletext/plugin-dnd Patch
@portabletext/plugin-emoji-picker Patch
@portabletext/plugin-input-rule Patch
@portabletext/plugin-list-index Patch
@portabletext/plugin-markdown-shortcuts Patch
@portabletext/plugin-one-line Patch
@portabletext/plugin-paste-link Patch
@portabletext/plugin-sdk-value Patch
@portabletext/plugin-table Patch
@portabletext/plugin-typeahead-picker Patch
@portabletext/plugin-typography Patch
@portabletext/toolbar Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
portable-text-editor-documentation Ready Ready Preview, Comment Jul 22, 2026 6:53pm
portable-text-example-basic Ready Ready Preview, Comment Jul 22, 2026 6:53pm
portable-text-playground Ready Ready Preview, Comment Jul 22, 2026 6:53pm

Request Review

@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

📦 Bundle Stats — @portabletext/editor

Compared against main (1bd850bd)

@portabletext/editor

Metric Value vs main (1bd850b)
Internal (raw) 807.4 KB +545 B, +0.1%
Internal (gzip) 155.1 KB +95 B, +0.1%
Bundled (raw) 1.42 MB +545 B, +0.0%
Bundled (gzip) 319.4 KB +91 B, +0.0%
Import time 55ms +1ms, +2.5%

@portabletext/editor/behaviors

Metric Value vs main (1bd850b)
Internal (raw) 467 B -
Internal (gzip) 207 B -
Bundled (raw) 424 B -
Bundled (gzip) 171 B -
Import time 1ms +0ms, +0.3%

@portabletext/editor/plugins

Metric Value vs main (1bd850b)
Internal (raw) 2.7 KB -
Internal (gzip) 894 B -
Bundled (raw) 2.5 KB -
Bundled (gzip) 827 B -
Import time 4ms +0ms, +0.5%

@portabletext/editor/selectors

Metric Value vs main (1bd850b)
Internal (raw) 82.7 KB -
Internal (gzip) 15.4 KB -
Bundled (raw) 78.4 KB -
Bundled (gzip) 14.3 KB -
Import time 4ms -0ms, -2.2%

@portabletext/editor/traversal

Metric Value vs main (1bd850b)
Internal (raw) 28.1 KB -
Internal (gzip) 5.6 KB -
Bundled (raw) 28.1 KB -
Bundled (gzip) 5.5 KB -
Import time 3ms +0ms, +2.3%

@portabletext/editor/utils

Metric Value vs main (1bd850b)
Internal (raw) 30.6 KB -
Internal (gzip) 6.4 KB -
Bundled (raw) 28.2 KB -
Bundled (gzip) 6.1 KB -
Import time 3ms -0ms, -2.9%

🗺️ . · ./behaviors · ./plugins · ./selectors · ./traversal · ./utils · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

📦 Bundle Stats — @portabletext/markdown

Compared against main (1bd850bd)

Metric Value vs main (1bd850b)
Internal (raw) 53.8 KB -
Internal (gzip) 9.8 KB -
Bundled (raw) 348.9 KB -
Bundled (gzip) 96.3 KB -
Import time 22ms -1ms, -4.9%

🗺️ View treemap · Artifacts

Details
  • Import time regressions over 10% are flagged with ⚠️
  • Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.

`validateValue` runs at sync ingress and can auto-resolve certain
invalid shapes (a child missing `_key`, empty `children`, unused
`markDefs`). The resolution existed only as patches: the changed-block
path emitted them outbound (mid-session, non-readOnly) while the raw,
un-repaired block proceeded into `updateBlock`/`replaceBlock`, and the
insert path dropped them entirely. The engine ended up holding the
un-repaired shape, a keyless child among them, which the engine's
addressing model cannot represent, and diverged from the document: the
emitted patch minted one key while engine normalization minted another
for the same node (observable in the old test pins, which expected the
second mint).

Apply the resolution to the block before it enters the engine, at both
call sites, via `applyAll`. The emitted patches are unchanged and now
agree with engine state: one key, minted once, on both sides. The
engine never holds the un-repaired shape.

Emission semantics are untouched: the insert path still emits nothing,
and the changed-block path still emits only mid-session for
non-readOnly editors. Test pins shift accordingly: the self-solving and
container-normalization suites now expect the first mint's key, and the
operation-stream scenario for empty-children blocks expects the
placeholder span inside the block's own insert operation rather than as
a separate normalization fix. The normalization-adjacency contract is
re-pinned on duplicate child keys, which have no ingress check and are
still repaired by normalization.
@christianhg
christianhg force-pushed the fix-sync-autoresolve-repair branch from 117a058 to c2e6232 Compare July 22, 2026 18:52
@christianhg
christianhg marked this pull request as ready for review July 22, 2026 18:53
@christianhg
christianhg enabled auto-merge (rebase) July 22, 2026 19:07
@christianhg
christianhg merged commit 5ddcc5b into main Jul 22, 2026
18 of 20 checks passed
@christianhg
christianhg deleted the fix-sync-autoresolve-repair branch July 22, 2026 19:08
@ecoscript ecoscript Bot mentioned this pull request Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant